Skip to content

Conversation

@raju-mechatronics
Copy link

Bug Fix

Problem

The json() method in context.go (line 504) was inconsistent with other response methods in how it sets the HTTP status code.

Current behavior:

func (c *context) json(code int, i any, indent string) error {
    c.writeContentType(MIMEApplicationJSON)
    c.response.Status = code  // ❌ Directly setting Status field
    return c.echo.JSONSerializer.Serialize(c, i, indent)
}

This approach directly sets the Status field instead of properly calling WriteHeader(), which bypasses header commitment and prevents warnings about header modifications after the status is set.

Solution

Updated the method to use c.response.WriteHeader(code) for consistency with other response methods:

func (c *context) json(code int, i any, indent string) error {
    c.writeContentType(MIMEApplicationJSON)
    c.response.WriteHeader(code)  // ✅ Properly calls WriteHeader
    return c. echo.JSONSerializer.Serialize(c, i, indent)
}

All other similar response methods already use WriteHeader():

  • jsonPBlob() - line 489: c.response.WriteHeader(code)
  • xml() - line 543: c.response.WriteHeader(code)
  • Blob() - line 578: c.response.WriteHeader(code)
  • JSONPBlob() - line 530: c.response.WriteHeader(code)

@raju-mechatronics
Copy link
Author

@aldas please take a look when you get a chance.

@aldas
Copy link
Contributor

aldas commented Jan 21, 2026

Hi, this is a "feature" that Echo uses. c.response.Status = code sets the code for status, which will be used with next responseWriter.Write call and until there have not been "Write" calls the echo.JSONSerializer.Serialize implementation can decide to use different status code.

I really do not want to touch this part as it could introduce subtle bugs for people that rely on this "feature".

@raju-mechatronics
Copy link
Author

Hi @aldas,

Thank you very much for taking the time to review and for the clear explanation — I really appreciate your insight!

I proposed this change because c.JSON() currently allows multiple calls in the same handler without any warning, unlike most other response methods (XML(), Blob(), etc.) that use WriteHeader() and correctly log "response already committed".

Minimal example showing the difference:

// /multiple-json → no warnings
	e.GET("/multiple", func(c echo.Context) error {
		responses := []Response{
			{ID: 1, Name: "first"},
			{ID: 2, Name: "second"},
			{ID: 3, Name: "third"},
		}
		c.JSON(200, responses[0])
		c.JSON(200, responses[1])
		c.JSON(200, responses[2])
		return nil
	})

// /multiple-xml → warns twice
	e.GET("/multiple_xml", func(c echo.Context) error {
		responses := []Response{
			{ID: 1, Name: "first"},
			{ID: 2, Name: "second"},
			{ID: 3, Name: "third"},
		}

		c.XML(200, responses[0])
		c.XML(200, responses[1])
		c.XML(200, responses[2])

		return nil
	})

the logs:

⇨ http server started on [::]:8080
method=GET, uri=/multiple, status=200
{"time":"2026-01-21T18:05:17.741384+06:00","level":"WARN","prefix":"echo","file":"response.go","line":"59","message":"response already committed"}
{"time":"2026-01-21T18:05:17.741707+06:00","level":"WARN","prefix":"echo","file":"response.go","line":"59","message":"response already committed"}
method=GET, uri=/multiple_xml, status=200

I thought aligning JSON() with the other methods would help catch accidental multiple-response mistakes more consistently.

@aldas
Copy link
Contributor

aldas commented Jan 21, 2026

Seems that in v5 this part has been changed. It is currently as

echo/context.go

Lines 454 to 458 in 096ce41

func (c *Context) json(code int, i any, indent string) error {
c.writeContentType(MIMEApplicationJSON)
c.response.WriteHeader(code)
return c.echo.JSONSerializer.Serialize(c, i, indent)
}

For history sake: c.response.Status = code was added in #1880 (v4.4.0)

I am not going to touch this part in v4 anymore - it works as it is and we are only doing security and bugfixes in v4. But I will check if v5 needs tinkering in that part.

@raju-mechatronics
Copy link
Author

raju-mechatronics commented Jan 21, 2026

okay. Thanks for the clarification.
I'll be in touch if I run into other issues or have another PR to contribute.

@aldas
Copy link
Contributor

aldas commented Jan 21, 2026

Hey @raju-mechatronics, thanks for taking the initiative here. Even though this PR didn’t get merged, it’s great to have people digging into the library’s internals and reviewing how things work. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants